docs: Clarify requirements about kids in the secrets.keys config
#4807
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sandhose
requested changes
Jul 22, 2025
There was a problem hiding this comment.
I think the 'arbitrary' here was meant to say 'any value you want, doesn't matter'. That's probably something that sounds about right in french, but not in english :)
I'd be happy to have the kid derived from a hash; the reason I didn't do it back then was that I couldn't really find a standard key fingerprinting mechanism, but admittedly I didn't look very far
sandhose
reviewed
Jul 23, 2025
docs/reference/configuration.md
Outdated
| The key can either be specified inline (with the `key` property), or loaded from a file (with the `key_file` property). | ||
| Each entry must have a unique `kid`, plus the key itself. | ||
| The `kid` can be any case-sensitive string value as long as it is unique to this list; | ||
| `kid` values don’t need to be stable across restarts. |
There was a problem hiding this comment.
That is actually not true? If we issue an id_token and change the kid of the key that was used to sign it, the client won't be able to verify it
There was a problem hiding this comment.
Ok, I hope this is better now?
sandhose
approved these changes
Jul 23, 2025
sandhose
changed the title
kids in the secrets.keys config
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The docs currently note that the
kidproperty specified forsecrets.keysitems has to be arbitrary. This PR removes that requirement and documents more loose ones instead.The JWT spec itself does have only few restrictions on KIDs. Having this additional requirement of arbitrary KIDs in MAS would disqualify the use of a hash function or a running number to create KIDs.
The PR also clarifies that it is ok for KIDs to change between restarts.
The suggested documentation would make automatic key management easier for me. I think these more loose requirements don’t break anything in MAS, but please correct me if I’m wrong.
PS: I’m thinking about making the
kidproperty optional and auto-generating it by MAS if missing, e.g., by hashing the corresponding key. Would you take such a PR?